Skip to content

Python: handle concurrent FileSystemAgentFileStore deletion - #8457

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 4 commits into
microsoft:mainfrom
ryo-whaletech:fix/filesystem-agent-file-store-concurrent-delete
Sep 28, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 4 commits into
microsoft:mainfrom
ryo-whaletech:fix/filesystem-agent-file-store-concurrent-delete

Conversation

@ryo-whaletech

@ryo-whaletech WhaleTech (ryo-whaletech) commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

AgentFileStore.delete() returns True when deletion occurs and False when the file does not exist. Separate FileSystemAgentFileStore instances that share a root can race on the same filesystem entry and violate that result contract.

Description & Review Guide

  • What are the major changes? Add a single process-local deletion lock around the existing file check and deletion, map only FileNotFoundError during deletion to False, and add deterministic cross-instance regressions for same-path and case-alias deletion.
  • What is the impact of these changes? Concurrent deletes of the same filesystem entry within one process, with no intervening write, produce one deletion result and one missing-file result. Existing directory/non-file semantics, path and symlink safety, and propagation of unrelated filesystem errors remain unchanged. This does not provide cross-process serialization or delete/write atomicity.
  • What do you want reviewers to focus on? The process-local locking scope, preservation of existing non-file behavior, narrow exception handling, and the regressions' ability to fail when synchronization is removed.

Related Issue

Fixes #8456

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI balanced review requested due to automatic review settings September 17, 2026 10:38
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Sep 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation preserves existing behavior while correctly covering the reported race with a focused regression test.

Pull request overview

Adds process-local synchronization to preserve FileSystemAgentFileStore.delete() result semantics during concurrent same-path deletion.

Changes:

  • Adds bounded striped locks around file checks and deletion.
  • Maps deletion-time FileNotFoundError to False.
  • Adds a deterministic cross-instance concurrency regression test.
File summaries
File Description
python/packages/core/agent_framework/_harness/_file_access.py Serializes same-stripe deletions and handles deletion races.
python/packages/core/tests/core/test_harness_file_access.py Tests concurrent deletion across store instances.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/core/agent_framework/_harness/_file_access.py Outdated
@ryo-whaletech
WhaleTech (ryo-whaletech) force-pushed the fix/filesystem-agent-file-store-concurrent-delete branch from 4754c48 to 8a6eb8e Compare September 18, 2026 12:14
Windows Path hashes case aliases the same way, so the native test
cannot find an alias with opposite hash parity. Skip only when that
precondition is unavailable and cover alias locking through delete()
on every CI platform.

Check that unlink-time FileNotFoundError returns False, PermissionError
propagates, and deleting a directory keeps its existing result.
Merged via the queue into microsoft:main with commit 789d514 Sep 28, 2026
45 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth — ca31bfef Deployed Sep 25, 2026 by ryo-whaletech via add_label #23761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: FileSystemAgentFileStore.delete can violate its result contract during concurrent deletion

5 participants